-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Bazel itself build under an output base with Unicode characters #24457
base: master
Are you sure you want to change the base?
Conversation
3d80129
to
abc6d14
Compare
I suspect that the RBE failure is due to the remote execution environment not having the |
I think that entails creating a new docker image for RBE (or finding a suitable existing one). @coeuvre, do you know how to do this? If this is blocking for 8.0.0, I suggest finding a way to either detect the existence of the locale in the test, or disable it when on RBE. |
The test is failing on other platforms, so I'll look into that. It's not a hard blocker for 8.0.0, it can equally well go into 8.1.0. |
This needs a fix in rules_java to properly set a UTF-8 environment for bootstrap actions: bazelbuild/rules_java#243 |
Copybara Import from #243 BEGIN_PUBLIC Build bootclasspath in a UTF-8 environment (#243) `java` and `javac` convert file and classpaths to absolute paths and thus require a UTF-8 locale to work under a path that contains non-ASCII characters. Unblocks bazelbuild/bazel#24457 Closes #243 END_PUBLIC COPYBARA_INTEGRATE_REVIEW=#243 from fmeum:utf8-environment 05813e4 PiperOrigin-RevId: 700695134 Change-Id: I2f5753720ec3c838a4dd8b6aabf1050c6935ef3d
This requires patching rules_graalvm with sgammon/rules_graalvm#514 to ensure the native image runs with `sun.jnu.encoding` set to `UTF-8` on Linux. Also enable unrelated tests after the recent java_tools update. Work towards #24444 Unblocks #24457 Closes #24565. PiperOrigin-RevId: 703409662 Change-Id: I44de4387de4a6da404436f5a35a2b27274122bbb
6a0811c
to
4c78604
Compare
9ad3d9a
to
ff647db
Compare
0ae309a
to
c51c9ea
Compare
The README.md in the java_tools zip archives has provenance info. Looks like v13.14 was built at 7d2509e |
That's convenient. I can now tell that it doesn't include the required a37d288. @hvadehra I filed bazelbuild/rules_java#265 as a replacement for bazelbuild/rules_java#250. |
# Conflicts: # src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@tjgq I had to resolve a non-trivial merge conflict with master. |
CI is finally green, thanks to everyone who was cutting releases (@iancha1992 @hvadehra)! |
@bazel-io fork 8.1.0 |
* Returns the path of a new temporary directory with the given prefix created under the given | ||
* parent path. This method is only supported by file system implementations that are backed by | ||
* the local file system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, I'm not a fan of adding more FileSystem
methods that only work on certain implementations; it's guaranteed to cause problems because we have a lot of them and it's not always immediately apparent which one we're dealing with (it might differ between production and testing, or depending on which flags you invoke Bazel with).
How difficult would it be to make it do something reasonable on any FileSystem
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could hand-roll an implementation that creates directories under /tmp
, assuming that any non-Unix system would use the current implementation. Would that address your concern?
Path#getInputStream()
, thus avoiding string encoding issues when constructing aFileInputStream
manually.Path#createTempDirectory
function is introduced to replace error-prone usages ofFiles.createTempDirectory
throughout the code base, which requires reencoding of both the arguments and the return value.argv0
inSubprocessBuilder
andWorkerMultiplexer
.info
output.Work towards #374
Fixes #24444